Add date subheaders to the RS comment list by nbradbury · Pull Request #23139 · wordpress-mobile/WordPress-Android

nbradbury · 2026-07-23T13:27:55Z

What

Adds date-group subheaders to the RS comment list matching the legacy comments list.

Testing

Enable the RS unified comments experimental feature and select an application-password site.
Open Comments and confirm rows are grouped under all-caps date headers ("Today", "Yesterday", a date, or a relative span), with a new header wherever the date label changes.

Interleave date-group headers into the rs comment list, matching the legacy list. A new withDateHeaders() groups consecutive comments by their existing relativeDate label (the same javaDateToTimeSpan value legacy groups by) and inserts a header row on each change; the LazyColumn renders headers and comments as distinct row types.

dangermattic · 2026-07-23T13:28:20Z

	1 Warning
⚠️	PR is not assigned to a milestone.

Generated by 🚫 Danger

wpmobilebot · 2026-07-23T13:36:27Z

📲 You can test the changes from this Pull Request in Jetpack Android by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Android
	Build Type	Debug
	Version	pr23139-ef34802
	Build Number	`1498`
	Application ID	`com.jetpack.android.prealpha`
	Commit	`ef34802`
	Installation URL	7fmg8peuv9ss8

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

wpmobilebot · 2026-07-23T13:36:47Z

📲 You can test the changes from this Pull Request in WordPress Android by scanning the QR code below to install the corresponding build.

	App Name	WordPress Android
	Build Type	Debug
	Version	pr23139-ef34802
	Build Number	`1498`
	Application ID	`org.wordpress.android.prealpha`
	Commit	`ef34802`
	Installation URL	108f3gnpt86co

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

codecov · 2026-07-23T13:39:44Z

Codecov Report

❌ Patch coverage is 40.54054% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.83%. Comparing base (e05eaf0) to head (ef34802).

Files with missing lines	Patch %	Lines
...ndroid/ui/commentsrs/screens/CommentsRsListItem.kt	0.00%	13 Missing ⚠️
...d/ui/commentsrs/screens/CommentsRsTabListScreen.kt	0.00%	8 Missing ⚠️
...rdpress/android/ui/commentsrs/CommentsRsListRow.kt	93.75%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            trunk   #23139   +/-   ##
=======================================
  Coverage   37.83%   37.83%           
=======================================
  Files        2344     2345    +1     
  Lines      127482   127515   +33     
  Branches    17703    17711    +8     
=======================================
+ Hits        48229    48244   +15     
- Misses      75296    75313   +17     
- Partials     3957     3958    +1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Key each date header by its label instead of the group's first comment id, so a header stays stable (no remove+add re-animation) when a newer comment is prepended into an existing date group. Labels are unique per contiguous group given the date sort. Drops the now-unneeded keyId field. - Uppercase the header label with the default locale, matching the legacy subheader's locale-aware textAllCaps (Kotlin's no-arg uppercase() is Locale.ROOT and mis-cases e.g. Turkish month names).

Reading Locale.getDefault() in a composable trips lint's NonObservableLocale (the header wouldn't recompose on a locale change). Read the locale from LocalConfiguration via ConfigurationCompat instead, matching the existing stats-card pattern, keeping the locale-aware uppercase.

…bheaders

adalpari · 2026-07-24T09:20:42Z

It looks like the new logic could end up in a possible crash. So, I would take a look:

CommentsRsTabListScreen.kt:151 — New LazyColumn key-uniqueness constraint can hard-crash where the legacy list only showed a duplicate header (PLAUSIBLE).

Header rows are keyed "header_${row.label}". LazyColumn throws IllegalArgumentException (“Key was already used”) if two rows resolve to the same key. The code relies on the invariant “comments are date-sorted, so a label maps to exactly one contiguous group.” That holds for normal server responses (date-descending pages), so this won’t fire in the common case — but it’s a new constraint: the legacy RecyclerView adapter tolerated duplicate SubHeader(label) values harmlessly. If the server ever returns comments out of strict date order (or a moderation/merge reorders such that two non-adjacent comments share an identical relative/absolute label), the label repeats non-contiguously → duplicate key → crash, where the old list just rendered a redundant header.

Failure scenario: an accumulated rawComments set (multi-page, distinctBy on append) whose order is not strictly monotonic by javaDateToTimeSpan label produces two DateHeader("…") with the same label → IllegalArgumentException on composition. Low probability but a hard crash. Worth a defensive measure (e.g. dedupe/uniquify header keys, or fall back gracefully) given the failure mode is a crash rather than a cosmetic glitch.

The header LazyColumn key relied on the invariant that date-sorted comments make each label one contiguous group. If the list ever arrived out of date order a label could repeat non-contiguously, producing a duplicate key -> LazyColumn IllegalArgumentException (a crash, where the legacy list only showed a redundant header). withDateHeaders now mints a guaranteed-unique key per header (still just the label in the normal case, disambiguated only on a non-contiguous repeat), so correctness no longer depends on the sort order.

nbradbury · 2026-07-24T11:32:15Z

It looks like the new logic could end up in a possible crash. So, I would take a look:

Once again I'm surprised that my Claude review didn't uncover this potential problem. Fixed in eeaf9d7.

…bheaders

adalpari

LGTM!

adalpari · 2026-07-24T12:04:00Z

It looks like the new logic could end up in a possible crash. So, I would take a look:

Once again I'm surprised that my Claude review didn't uncover this potential problem. Fixed in eeaf9d7.

I'm in a mode where I always expect new potential issues when someone else reviews my PRs, using the exact ask to Claude I did like 2 or 3 times before opening it. 😅

nbradbury added the Comments label Jul 23, 2026

github-advanced-security AI found potential problems Jul 23, 2026

View reviewed changes

Comment thread WordPress/src/main/java/org/wordpress/android/ui/commentsrs/screens/CommentsRsListItem.kt Fixed

This comment was marked as resolved.

Sign in to view

nbradbury added 2 commits July 23, 2026 10:35

Merge remote-tracking branch 'origin/trunk' into issue/rs-comments-su…

4f45b2d

…bheaders

nbradbury marked this pull request as ready for review July 23, 2026 15:04

nbradbury requested a review from adalpari July 23, 2026 15:04

Merge remote-tracking branch 'origin/trunk' into issue/rs-comments-su…

ef34802

…bheaders

adalpari approved these changes Jul 24, 2026

View reviewed changes

nbradbury merged commit 33fe249 into trunk Jul 24, 2026
23 checks passed

nbradbury deleted the issue/rs-comments-subheaders branch July 24, 2026 12:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add date subheaders to the RS comment list - #23139

Add date subheaders to the RS comment list#23139
nbradbury merged 6 commits into
trunkfrom
issue/rs-comments-subheaders

nbradbury commented Jul 23, 2026 •

edited

Loading

Uh oh!

dangermattic commented Jul 23, 2026 •

edited

Loading

Uh oh!

wpmobilebot commented Jul 23, 2026 •

edited

Loading

Uh oh!

wpmobilebot commented Jul 23, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

This comment was marked as resolved.

adalpari commented Jul 24, 2026

Uh oh!

nbradbury commented Jul 24, 2026

Uh oh!

adalpari left a comment

Uh oh!

adalpari commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

nbradbury commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Testing

Uh oh!

dangermattic commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

This comment was marked as resolved.

adalpari commented Jul 24, 2026

Uh oh!

nbradbury commented Jul 24, 2026

Uh oh!

adalpari left a comment

Choose a reason for hiding this comment

Uh oh!

adalpari commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nbradbury commented Jul 23, 2026 •

edited

Loading

dangermattic commented Jul 23, 2026 •

edited

Loading

wpmobilebot commented Jul 23, 2026 •

edited

Loading

wpmobilebot commented Jul 23, 2026 •

edited

Loading

codecov Bot commented Jul 23, 2026 •

edited

Loading